Skip to content

Add ilev as a default output in all EAMxx output streams#3091

Merged
AaronDonahue merged 4 commits intomasterfrom
aarondonahue/io/add_ilev_to_output
Nov 12, 2024
Merged

Add ilev as a default output in all EAMxx output streams#3091
AaronDonahue merged 4 commits intomasterfrom
aarondonahue/io/add_ilev_to_output

Conversation

@AaronDonahue
Copy link
Contributor

@AaronDonahue AaronDonahue commented Nov 4, 2024 •
edited by mahf708
Loading

This commit follows the example for the geometry data for 'lev' and adds 'ilev'. The variable 'ilev' will now be a default variable in all EAMxx output similar to what is currently done with lat/lon, hyam/hybm, hyai/hybi and lev.

Fixes #3022

@AaronDonahue
Add ilev as a default output in all EAMxx output streams
f72cc87 
This commit follows the example for the geometry data for 'lev'
and adds 'ilev'.  The variable 'ilev' will now be a default variable
in all EAMxx output similar to what is currently done with lat/lon,
hyam/hybm, hyai/hybi and lev.

Addresses #3022
@AaronDonahue AaronDonahue added New Variables New variables added, B4B in old variables I/O labels Nov 4, 2024
@AaronDonahue AaronDonahue requested a review from bartgol November 4, 2024 20:07
@mahf708
Copy link
Contributor

mahf708 commented Nov 5, 2024

@bartgol @AaronDonahue how about we extract the logic for this type of op such that it can be generalized (and tidied up)? Something akin to add_dim_info(details) where the details would have the final (not intermediate) naming, values, etc. for everything to be added

@bartgol if that seems feasible, I can probably impl ...

@bartgol
Copy link
Contributor

bartgol commented Nov 7, 2024

@bartgol @AaronDonahue how about we extract the logic for this type of op such that it can be generalized (and tidied up)? Something akin to add_dim_info(details) where the details would have the final (not intermediate) naming, values, etc. for everything to be added

@bartgol if that seems feasible, I can probably impl ...

Where would you add this? Inside I/O? Or as a method of the grid? Generally speaking, I think it should be feasible. It could be as simple as offering a small method in the grid class, which internally simply calls "set_geo_data"...

@AaronDonahue
Copy link
Contributor Author

AaronDonahue commented Nov 8, 2024 •
edited
Loading

I'm afraid of this PR getting lost in limbo. Would it make sense to merge the PR as is now and clean up once we have the impl discussed above? @bartgol @mahf708

@mahf708
Copy link
Contributor

mahf708 commented Nov 9, 2024 •
edited
Loading

I'm afraid of this PR getting lost in limbo. Would it make sense to merge the PR as is now and clean up once we have the impl discussed above? @bartgol @mahf708

Yep! (Note it seems you have real fails going on the tests though, so these need to be fixed)

@mergify
Copy link
Contributor

mergify bot commented Nov 11, 2024 •
edited
Loading

Merge Protections

Your pull request matches the following merge protections and will not be merged until they are valid.

🟢 Enforce checks passing

Wonderful, this rule succeeded.

Make sure that checks are not failing on the PR, and reviewers approved

  • #approved-reviews-by >= 1
  • #changes-requested-reviews-by == 0
  • any of:
    • check-success="gcc-openmp / dbg"
    • check-skipped="gcc-openmp / dbg"
  • any of:
    • check-success="gcc-openmp / sp"
    • check-skipped="gcc-openmp / sp"
  • any of:
    • check-success="gcc-openmp / fpe"
    • check-skipped="gcc-openmp / fpe"
  • any of:
    • check-success="gcc-openmp / opt"
    • check-skipped="gcc-openmp / opt"
  • any of:
    • check-success="gcc-cuda / dbg"
    • check-skipped="gcc-cuda / dbg"
  • any of:
    • check-success="gcc-cuda / sp"
    • check-skipped="gcc-cuda / sp"
  • any of:
    • check-success="gcc-cuda / opt"
    • check-skipped="gcc-cuda / opt"
  • any of:
    • check-success="cpu-gcc / ERS_Ln9.ne4_ne4.F2000-SCREAMv1-AQP1.scream-output-preset-2"
    • check-skipped="cpu-gcc / ERS_Ln9.ne4_ne4.F2000-SCREAMv1-AQP1.scream-output-preset-2"
  • any of:
    • check-success="cpu-gcc / ERS_P16_Ln22.ne30pg2_ne30pg2.FIOP-SCREAMv1-DP.scream-dpxx-arm97"
    • check-skipped="cpu-gcc / ERS_P16_Ln22.ne30pg2_ne30pg2.FIOP-SCREAMv1-DP.scream-dpxx-arm97"
  • any of:
    • check-success="cpu-gcc / ERS_Ln22.ne4pg2_ne4pg2.F2010-SCREAMv1.scream-small_kernels--scream-output-preset-5"
    • check-skipped="cpu-gcc / ERS_Ln22.ne4pg2_ne4pg2.F2010-SCREAMv1.scream-small_kernels--scream-output-preset-5"
  • any of:
    • check-success="cpu-gcc / SMS_D_Ln5.ne4pg2_oQU480.F2010-SCREAMv1-MPASSI.scream-mam4xx-all_mam4xx_procs"
    • check-skipped="cpu-gcc / SMS_D_Ln5.ne4pg2_oQU480.F2010-SCREAMv1-MPASSI.scream-mam4xx-all_mam4xx_procs"
  • any of:
    • check-skipped=cpu-gcc
    • check-success=cpu-gcc

@AaronDonahue AaronDonahue merged commit aca29bc into master Nov 12, 2024
@AaronDonahue AaronDonahue deleted the aarondonahue/io/add_ilev_to_output branch November 12, 2024 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bartgol bartgol bartgol approved these changes

@mahf708 mahf708 mahf708 approved these changes

Assignees

No one assigned

Labels

I/O New Variables New variables added, B4B in old variables

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ilev not being written to output files

3 participants

@AaronDonahue @mahf708 @bartgol